Skip to content

[WRONG BRANCH] fix(claude): isolate passthrough credentials - #73

Closed
luvs01 wants to merge 1 commit into
mainfrom
codex/fix-claude-passthrough-vulnerability
Closed

[WRONG BRANCH] fix(claude): isolate passthrough credentials#73
luvs01 wants to merge 1 commit into
mainfrom
codex/fix-claude-passthrough-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent native Anthropic passthrough from accidentally forwarding OpenCodex admission tokens via Authorization or X-Api-Key, which could leak management/data-plane credentials to upstream providers.
  • Remove ambiguity between proxy admission headers and provider-owned credentials so an Anthropic upstream cannot obtain a token that gates /api/* management routes.

Description

  • Hardened the native Anthropic passthrough in src/server/claude-messages.ts by importing auth helpers and changing credential detection so Authorization/X-Api-Key values that match OpenCodex admission secrets are ignored as provider credentials and are not forwarded upstream.
  • Require the dedicated X-OpenCodex-API-Key on non-loopback listeners for native passthrough requests, so non-loopback passthrough only activates when admission is presented via the dedicated header.
  • Strip any admission-secret-looking values found in Authorization or X-Api-Key before copying headers to the upstream Anthropic request to ensure proxy admission secrets are never forwarded.
  • Added a regression test tests/claude-native-passthrough.test.ts that verifies ambiguous legacy-header usage is rejected and the dedicated-header path still succeeds, and updated the native passthrough docs (English/Korean/Simplified Chinese) to document header handling and the non-loopback requirement.

Testing

  • Ran bun run typecheck and it passed.
  • Ran the focused tests bun run test -- tests/claude-native-passthrough.test.ts and they passed (11 tests, 0 failures).
  • Ran bun run privacy:scan and the scan passed after replacing test fixture tokens with non-sensitive placeholders.
  • Performed a broader bun run test run that exercised many suites locally and observed passing results for the exercised tests, but a full docs-site build (cd docs-site && bun install --frozen-lockfile && bun run build) was blocked by external npm registry 403 errors so the site build could not be completed in this environment.

Summary by CodeRabbit

  • Bug Fixes

    • Improved native Claude passthrough authentication and credential handling.
    • Proxy admission credentials are no longer forwarded to upstream providers.
    • Exposed listeners now require the dedicated x-opencodex-api-key header.
    • Genuine provider credentials continue to be forwarded end-to-end.
  • Documentation

    • Updated English, Korean, and Chinese Claude passthrough guides to explain authentication requirements and header handling.
  • Tests

    • Added coverage for rejecting ambiguous credentials and preventing proxy credentials from reaching providers.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

⏳ DRAFT

  • wrong target branch (main); retarget to dev.

What to do

  • Retarget this PR to dev — all contributions go to dev.

Its title has been prefixed with [WRONG BRANCH].
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@github-actions github-actions Bot changed the title fix(claude): isolate passthrough credentials [WRONG BRANCH] fix(claude): isolate passthrough credentials Aug 8, 2026
@github-actions github-actions Bot added the bug Something isn't working label Aug 8, 2026
@github-actions
github-actions Bot marked this pull request as draft August 8, 2026 01:19
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Native Claude passthrough now separates proxy admission authentication from Anthropic provider credentials. Exposed listeners require x-opencodex-api-key, and proxy credentials are removed before upstream forwarding. Tests and multilingual documentation cover the behavior.

Changes

Native Claude passthrough authentication

Layer / File(s) Summary
Admission authentication and credential detection
src/server/claude-messages.ts
At lines 31 and 98–114, passthrough authentication excludes OpenCodex admission secrets from provider credential detection. Exposed listeners require a valid x-opencodex-api-key.
Upstream credential filtering
src/server/claude-messages.ts
At lines 335–341, forwarding removes matching proxy credentials from authorization and x-api-key, while preserving other allowed headers.
Regression coverage and documentation
tests/claude-native-passthrough.test.ts, docs-site/src/content/docs/guides/claude-code.md, docs-site/src/content/docs/ko/guides/claude-code.md, docs-site/src/content/docs/zh-cn/guides/claude-code.md
Tests cover rejected ambiguous credentials, dedicated admission authentication, upstream filtering, and cleanup. The guides document the updated authentication and forwarding rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant NativePassthrough
  participant AdmissionValidation
  participant AnthropicUpstream
  Client->>NativePassthrough: Submit native Claude request
  NativePassthrough->>AdmissionValidation: Validate x-opencodex-api-key when listener is exposed
  AdmissionValidation-->>NativePassthrough: Return admission result
  NativePassthrough->>NativePassthrough: Remove proxy admission credentials
  NativePassthrough->>AnthropicUpstream: Forward provider credentials and allowed headers
Loading

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main change: isolating credentials for Claude passthrough requests.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-claude-passthrough-vulnerability

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/claude-native-passthrough.test.ts`:
- Around line 230-245: Add a second successful dedicated passthrough request in
this test, using the admission secret in authorization, a provider credential in
x-api-key, and the valid x-opencodex-api-key. Assert the request succeeds and
the captured upstream request removes authorization while retaining the provider
x-api-key; keep the existing x-opencodex-api-key filtering assertions.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: afaab7aa-54f5-4aa3-a7a6-61545bc20d4f

📥 Commits

Reviewing files that changed from the base of the PR and between 2468502 and 2415c39.

📒 Files selected for processing (5)
  • docs-site/src/content/docs/guides/claude-code.md
  • docs-site/src/content/docs/ko/guides/claude-code.md
  • docs-site/src/content/docs/zh-cn/guides/claude-code.md
  • src/server/claude-messages.ts
  • tests/claude-native-passthrough.test.ts

Comment on lines +230 to +245
const dedicated = await globalThis.fetch(url, {
method: "POST",
headers: {
"content-type": "application/json",
"x-opencodex-api-key": "sk-ant-api03-key",
"authorization": "Bearer sk-ant-oat01-tst",
"x-api-key": "sk-ant-api03-key",
},
body: JSON.stringify(claudeBody()),
});
expect(dedicated.status).toBe(200);
await dedicated.text();
expect(captured).toHaveLength(1);
expect(captured[0].headers.get("authorization")).toBe("Bearer sk-ant-oat01-tst");
expect(captured[0].headers.get("x-api-key")).toBeNull();
expect(captured[0].headers.get("x-opencodex-api-key")).toBeNull();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Add a successful passthrough case for authorization filtering.

Lines 243-245 only verify removal of an admission secret from x-api-key. The rejected request does not enter anthropicNativePassthrough, so it does not test the authorization branch at src/server/claude-messages.ts lines 337-340.

Send a second dedicated request with the admission secret in authorization, a provider credential in x-api-key, and the valid x-opencodex-api-key. Assert that the upstream receives no authorization header and retains the provider x-api-key.

Proposed regression case
+    const authorizationAdmission = await globalThis.fetch(url, {
+      method: "POST",
+      headers: {
+        "content-type": "application/json",
+        "x-opencodex-api-key": "sk-ant-api03-key",
+        "authorization": "Bearer sk-ant-api03-key",
+        "x-api-key": "sk-ant-oat01-tst",
+      },
+      body: JSON.stringify(claudeBody()),
+    });
+    expect(authorizationAdmission.status).toBe(200);
+    await authorizationAdmission.text();
+    expect(captured).toHaveLength(2);
+    expect(captured[1].headers.get("authorization")).toBeNull();
+    expect(captured[1].headers.get("x-api-key")).toBe("sk-ant-oat01-tst");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const dedicated = await globalThis.fetch(url, {
method: "POST",
headers: {
"content-type": "application/json",
"x-opencodex-api-key": "sk-ant-api03-key",
"authorization": "Bearer sk-ant-oat01-tst",
"x-api-key": "sk-ant-api03-key",
},
body: JSON.stringify(claudeBody()),
});
expect(dedicated.status).toBe(200);
await dedicated.text();
expect(captured).toHaveLength(1);
expect(captured[0].headers.get("authorization")).toBe("Bearer sk-ant-oat01-tst");
expect(captured[0].headers.get("x-api-key")).toBeNull();
expect(captured[0].headers.get("x-opencodex-api-key")).toBeNull();
const dedicated = await globalThis.fetch(url, {
method: "POST",
headers: {
"content-type": "application/json",
"x-opencodex-api-key": "sk-ant-api03-key",
"authorization": "Bearer sk-ant-oat01-tst",
"x-api-key": "sk-ant-api03-key",
},
body: JSON.stringify(claudeBody()),
});
expect(dedicated.status).toBe(200);
await dedicated.text();
expect(captured).toHaveLength(1);
expect(captured[0].headers.get("authorization")).toBe("Bearer sk-ant-oat01-tst");
expect(captured[0].headers.get("x-api-key")).toBeNull();
expect(captured[0].headers.get("x-opencodex-api-key")).toBeNull();
const authorizationAdmission = await globalThis.fetch(url, {
method: "POST",
headers: {
"content-type": "application/json",
"x-opencodex-api-key": "sk-ant-api03-key",
"authorization": "Bearer sk-ant-api03-key",
"x-api-key": "sk-ant-oat01-tst",
},
body: JSON.stringify(claudeBody()),
});
expect(authorizationAdmission.status).toBe(200);
await authorizationAdmission.text();
expect(captured).toHaveLength(2);
expect(captured[1].headers.get("authorization")).toBeNull();
expect(captured[1].headers.get("x-api-key")).toBe("sk-ant-oat01-tst");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/claude-native-passthrough.test.ts` around lines 230 - 245, Add a second
successful dedicated passthrough request in this test, using the admission
secret in authorization, a provider credential in x-api-key, and the valid
x-opencodex-api-key. Assert the request succeeds and the captured upstream
request removes authorization while retaining the provider x-api-key; keep the
existing x-opencodex-api-key filtering assertions.

Source: Path instructions

@luvs01

luvs01 commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

Superseded by the current-dev upstream implementation in lidge-jun#1536. The replacement preserves the credential-isolation intent, adds listener-effective policy handling, fails closed on duplicate credential headers, and includes the inverse provider x-api-key regression requested by CodeRabbit. Closing this stale wrong-branch draft without deleting its branch.

@luvs01 luvs01 closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant